Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(feat) Query builder #985

Merged
merged 89 commits into from
Jan 10, 2025
Merged

(feat) Query builder #985

merged 89 commits into from
Jan 10, 2025

Conversation

thomasht86
Copy link
Collaborator

@thomasht86 thomasht86 commented Dec 4, 2024 •
edited
Loading

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Consider this a first draft, and I can go through it tomorrow.

To discuss:

  • Should we allow for using kwargs for annotations - good for some, but many annotations are camelCase - does not good in python.
  • Rename Q and G- to what?

TODO:

  • Integration test for grouping

@thomasht86
add poc draft
805f3a3
@thomasht86
rename ui to userQuery
5bbd8e7
@thomasht86
nn operators - not passing
8cd21f3
@thomasht86
start with annotations
8b8297f
@thomasht86
more tests
6fce39d
@thomasht86
rest of methods 🚀 passing tests ✅
c58a8a4
@thomasht86
unify test classes
d749f54
@thomasht86
Merge branch 'master' into thomasht86/qbuilder-poc
09a3fb5
@thomasht86
restructure
b4bf206
@thomasht86
add integration test
511bb71
@thomasht86
rename Field to Queryfield
bc49b31
@thomasht86
use unit tests for integration tests
26e8c1d
@thomasht86
refacotring of unit tests
1f9c40b
@thomasht86
simplify
d0371e0
@thomasht86
add dunder methods to remove build
2eb7f63
@thomasht86
Merge branch 'master' into thomasht86/qbuilder-poc
268616c
@thomasht86
no index annotation on userQuery
b60c6cf
@thomasht86
more integration tests
8538efd
@thomasht86
more unit tests
41f70ec
@thomasht86
clean userquery
cb30290
@thomasht86
add userinput
a336bee
@thomasht86
userinput integration
10cc0b3
@thomasht86
no parenthesis annotations
52b609e
@thomasht86
predicate
e1b3cb7
@thomasht86
annotation formatting and predicate
21c8954
@thomasht86
wand
634513c
@thomasht86
timeout
f791d49
@thomasht86
exclude markdown files in resources
a191796
@thomasht86
Merge branch 'master' into thomasht86/qbuilder-poc
4ddef4d
@thomasht86
add fuzzy integration test
86c85af
@thomasht86
Copy link
Collaborator Author

Discovered more grouping functions that should be added (and tests for) before merging. :D
https://docs.vespa.ai/en/reference/grouping-syntax.html

@thomasht86 thomasht86 marked this pull request as ready for review January 10, 2025 10:01
@thomasht86
Copy link
Collaborator Author

Ready for another review now @glebashnik 🙏

glebashnik
glebashnik approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@glebashnik glebashnik left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, both design wise and tests.
I setup env and ran tests in Python 2.12 3.12.
For future improvement, it would be great to be able to run tests in different Python versions. Tools like Nox can help with this.

@thomasht86
Copy link
Collaborator Author

thomasht86 commented Jan 10, 2025 •
edited
Loading

Looks very good, both design wise and tests. I setup env and ran tests in Python 2.12. For future improvement, it would be great to be able to run tests in different Python versions. Tools like Nox can help with this.

Thanks. Assume you mean 3.12? 😅 We do run unit tests on matrix of Python 3.8-3.12 and macos, ubuntu, windows.
Doing it for all would take a lot of time and cost a lot, but maybe we could do some of the integration tests at least.

@thomasht86 thomasht86 merged commit a09afbe into master Jan 10, 2025
48 checks passed
@thomasht86 thomasht86 deleted the thomasht86/qbuilder-poc branch January 10, 2025 17:24
@thomasht86 thomasht86 mentioned this pull request Jan 13, 2025
Closed
@glebashnik
Copy link
Contributor

Right. Found the GH job. All good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glebashnik glebashnik glebashnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thomasht86 @glebashnik